Support arithmetic operations for dense_vectors: scalar version#141060
Support arithmetic operations for dense_vectors: scalar version#141060kkharbas wants to merge 10 commits intoelastic:mainfrom
Conversation
carlosdelest
left a comment
There was a problem hiding this comment.
Hey @kkharbas ! I don't think we need to have a
dense_vector + field
so we operate on each dense_vector field over each scalar field on the other column, but:
dense_vector + scalar constant
so we use the same scalar constant to operate on every dense_vector field row.
So, we could do something like:
EVAL my_negated_vectors = my_vector_field * -1
or
EVAL my_added_vectors = my_vector_field + 3.0
Right now I don't see the use case for operating on a row by row basis for scalars - and I think in any case using a single scalar constant would be a more frequent use case, and simpler to implement for now.
We may come back to this implementation, but I think for now we should reduce the scope and look for single numeric constants to be applied.
What do you think?
That makes total sense. I was just focussing on completeness of the operations but usability wise its fair to only support |
685c7a9 to
834ba7d
Compare
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
Adds support for arithmetic operations - '+', '-', '*' and '/' when one operands is a dense_vector and other is scalar. Performs the arithmetic operation on each element of the dense_vector against the scalar operand. Closes elastic#140538
834ba7d to
f70c477
Compare
carlosdelest
left a comment
There was a problem hiding this comment.
I have a question on the approach - instead of dealing separately with scalar operands in a separate class, convert them via an ExpressionEvaluator to dense vectors so we can operate on them with the current infra.
...arch/xpack/esql/expression/predicate/operator/arithmetic/DenseVectorArithmeticOperation.java
Outdated
Show resolved
Hide resolved
...arch/xpack/esql/expression/predicate/operator/arithmetic/DenseVectorArithmeticOperation.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Having a specific class for handling operating with doubles looks like something we should try to avoid.
I wonder about creating a ToFloat internal function for dealing with this, similar to other conversion functions like ToDouble. This would allow us to use ToFloat as part of the evaluator chain that the factory creates when we find a numeric type that we want to convert to floats, and then deal with doing float to float operations.
WDYT?
There was a problem hiding this comment.
I wonder if we can avoid having a specific evaluator for doubles, and adding a specific DenseVectorBinaryEvaluator.
I was thinking on having a ExpressionEvaluator that would convert a numeric block into a dense_vector. That way, we could wrap the numeric block using this converter before it is processed by the DenseVectorsEvaluator, and thus reducing the problem to an operation on two dense vectors which we already have solved.
WDYT?
I had given this approach some consideration but ran into hiccups. I will take a fresh look again at this approach. |
I tried out this approach of using existing conversion evaluators, e.g. Few thoughts and questions on this,
|
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
test this please |
|
Hi @kkharbas, I've created a changelog YAML for you. |
Adds support for arithmetic operations - '+', '-', '*' and '/' when one operand is a dense_vector and other is scalar.
Performs the arithmetic operation on each element of the dense_vector against the scalar operand. The scalar operand should be a constant value, scalar fields are not supported.
✔️
dense_vector + scalar constant❌
dense_vector + scalar fieldCloses #140538